Skip to content

fix: return protocol errors for invalid tool args#2163

Open
he-yufeng wants to merge 1 commit into
modelcontextprotocol:mainfrom
he-yufeng:fix/tool-argument-protocol-error
Open

fix: return protocol errors for invalid tool args#2163
he-yufeng wants to merge 1 commit into
modelcontextprotocol:mainfrom
he-yufeng:fix/tool-argument-protocol-error

Conversation

@he-yufeng
Copy link
Copy Markdown

Summary

  • let invalid tool arguments surface as JSON-RPC InvalidParams errors
  • keep actual tool execution failures wrapped as CallToolResult isError responses
  • add an in-memory regression test for schema-invalid tools/call arguments

To verify

  • pnpm --filter @modelcontextprotocol/server test -- test/server/mcp.compat.test.ts
  • pnpm --filter @modelcontextprotocol/server typecheck
  • pnpm --filter @modelcontextprotocol/server lint
  • git diff --check

Fixes #2162

@he-yufeng he-yufeng requested a review from a team as a code owner May 28, 2026 09:17
@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented May 28, 2026

🦋 Changeset detected

Latest commit: 5ef13fd

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@modelcontextprotocol/server Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented May 28, 2026

Open in StackBlitz

@modelcontextprotocol/client

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/client@2163

@modelcontextprotocol/codemod

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/codemod@2163

@modelcontextprotocol/server

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/server@2163

@modelcontextprotocol/server-legacy

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/server-legacy@2163

@modelcontextprotocol/express

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/express@2163

@modelcontextprotocol/fastify

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/fastify@2163

@modelcontextprotocol/hono

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/hono@2163

@modelcontextprotocol/node

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/node@2163

commit: 5ef13fd

@he-yufeng he-yufeng force-pushed the fix/tool-argument-protocol-error branch from d683549 to 7641e96 Compare May 28, 2026 12:08
@he-yufeng
Copy link
Copy Markdown
Author

CI note: I checked the remaining red Node 20 job. The invalid-argument coverage added in this PR passes in that run; the failure is in test/server/cloudflareWorkers.test.ts with a Miniflare network connection lost error, which is outside the touched server validation path. I also tried to rerun the failed job, but GitHub requires repo admin rights for that.

@he-yufeng he-yufeng force-pushed the fix/tool-argument-protocol-error branch from 7641e96 to e814d61 Compare June 6, 2026 20:56
@he-yufeng
Copy link
Copy Markdown
Author

Rebased on current main, resolved the test conflicts after the experimental tasks removal, added the missing @modelcontextprotocol/server patch changeset, and force-pushed the branch.

Validated locally:

  • pnpm changeset status --since origin/main (@modelcontextprotocol/server patch)
  • pnpm --filter @modelcontextprotocol/server typecheck
  • pnpm --filter @modelcontextprotocol/server lint
  • pnpm --filter @modelcontextprotocol/test-integration lint
  • pnpm --filter @modelcontextprotocol/test-integration test -- test/server/mcp.test.ts test/standardSchema.test.ts (144 passed)
  • git diff --check origin/main..HEAD

The pre-push hook also completed the repo-wide build, typecheck, and lint steps successfully.

@he-yufeng he-yufeng force-pushed the fix/tool-argument-protocol-error branch from e814d61 to 5ef13fd Compare June 6, 2026 23:21
@he-yufeng
Copy link
Copy Markdown
Author

Rebased and force-pushed the branch.

I also updated the e2e contract expectations so invalid tool arguments now assert JSON-RPC -32602 instead of an isError tool result. Targeted validation passed:

  • pnpm --filter @modelcontextprotocol/server exec vitest run test/server/mcp.compat.test.ts
  • pnpm --filter @modelcontextprotocol/test-e2e test -- scenarios/tools.test.ts
  • pnpm --filter @modelcontextprotocol/test-e2e test -- scenarios/standard-schema.test.ts
  • pnpm --filter @modelcontextprotocol/test-e2e test -- scenarios/validation.test.ts
  • pnpm --filter @modelcontextprotocol/server typecheck
  • pnpm --filter @modelcontextprotocol/server lint
  • pnpm --filter @modelcontextprotocol/server build
  • pnpm --filter @modelcontextprotocol/test-e2e typecheck
  • pnpm --filter @modelcontextprotocol/test-e2e lint
  • git diff --check origin/main..HEAD

One combined e2e invocation hit a Vitest worker process exit after the relevant files had already run; rerunning the scenarios individually passed, so I split the validation above. The push hook also completed the repo-level build/typecheck/lint successfully.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

invalid argument should throw protocol error, rather than tool execution error

1 participant